Skip to content

Add unit tests for file_util presigned URL logic HHS#11647 - #4

Open
davidhuanggg wants to merge 1 commit into
HHS:mainfrom
davidhuanggg:issue#11647-file-util-presigned-url-tests-prior-to-repo-move
Open

Add unit tests for file_util presigned URL logic HHS#11647#4
davidhuanggg wants to merge 1 commit into
HHS:mainfrom
davidhuanggg:issue#11647-file-util-presigned-url-tests-prior-to-repo-move

Conversation

@davidhuanggg

@davidhuanggg davidhuanggg commented Aug 14, 2026

Copy link
Copy Markdown

Summary

Fixes #11647 HHS/simpler-grants-gov#11647

Changes proposed

Added:

  • Add unit tests for pre_sign_upload POST policy construction (Content-Type, metadata pinning, content-length-range from FileConfig, IfNoneMatch)
  • Add unit tests for pre_sign_file_location (bucket/key parsing, localhost endpoint override, non-default duration)
  • Add unit tests for presign_or_s3_cdnify_url CDN vs presign branching and non-s3:// ValueError
  • Add small test helpers to decode the signed policy and assert URL targets without checking signature values

Context for reviewers

  • These three helpers (pre_sign_upload, pre_sign_file_location, presign_or_s3_cdnify_url) were mostly only covered indirectly via API route tests. The security-relevant pieces especially pre_sign_upload's POST policy conditions weren't pinned at the util layer, so a refactor of the conditions list could slip through.

  • This PR adds direct unit tests in test_file_util.py that assert on policy structure and parsed URL query params rather than full signed URL / signature strings, so they stay stable across botocore signing changes.

  • Test only. make test passes in backend/grants_shared.

Validation steps

Local Reproduction

cd backend
cd grants_shared
make test

Output

============================= test session starts ==============================
platform linux -- Python 3.14.6, pytest-9.0.3, pluggy-1.6.0
rootdir: /grants_shared
configfile: pyproject.toml
testpaths: tests
plugins: Faker-40.15.0
collected 463 items / 15 deselected / 448 selected  
tests/grants_shared/util/test_string_utils.py ...................        [100%]

===================== 448 passed, 15 deselected in 12.70s ======================

Acceptance criteria

  • Tests for pre_sign_upload policy construction:
    • Content-Type appears in both Fields and Conditions
    • each metadata entry becomes an x-amz-meta-<key> field and a pinned condition
    • content-length-range condition is [1, max_file_upload_size_bytes] and tracks FileConfig, not a hardcoded number
    • include_if_none_match=True adds {"IfNoneMatch": "*"}; default (False) does not
    • returned dict has both url and fields, and fields carries the signed policy/signature
  • Tests for pre_sign_file_location:
    • bucket and key are taken from the s3:// path via split_s3_url (nested keys, keys with spaces/special chars)
    • the localhost:9090 override applies when aws_s3_endpoint_url is set, and the URL is left untouched when it is not
    • a non-default presigned_s3_duration is reflected in X-Amz-Expires (existing test covers the 900s default)
  • Tests for presign_or_s3_cdnify_url branching:
    • cdn_url set → returns a CDN URL, no presigning
    • cdn_url unset → returns a presigned URL
    • non-s3:// path with cdn_url set → raises ValueError
  • Tests live in backend/grants_shared/tests/grants_shared/util/test_file_util.py, use the existing mock_s3_bucket / s3_config fixtures, and parametrize where cases are shape-identical
  • No test asserts on a full signed URL string or signature value — assert on parsed query params and policy structure
  • make test passes in backend/grants_shared

Original PR linked prior to the repo move that will be closed:

HHS/simpler-grants-gov#11700

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant